-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Inigo dev #118
Conversation
inigoval
commented
May 23, 2024
- Add timm ViT fine-tuning support
- Add sklearn linear regression sanity check to classification finetuning (TODO: add to regression also)
- Some formatting also
zoobot/pytorch/training/finetune.py
Outdated
self.encoder = encoder | ||
# work out encoder dim 'manually' | ||
self.encoder_dim = define_model.get_encoder_dim(self.encoder) | ||
if isinstance(self.encoder, timm.models.VisionTransformer): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO for me, I later found out this actually works on all timm models so should add a check like this for everything
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to read attrs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to read attrs
zoobot/pytorch/training/finetune.py
Outdated
|
||
# Sanity check embeddings with linear evaluation first | ||
def on_train_start(self) -> None: | ||
with torch.no_grad(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense for you and I but I'm not sure it should run by default for Zoobot. I think it may confuse astronomers who will now get two separate performance measurements for linear eval. I'm also a bit worried about the potential memory use from encoding everything. Maybe we can just wrap this in a new arg to ZoobotAbstract, linear_sanity_check=False, and an if statement/func here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to if statement